feat: Package the extension. #79
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughA GitHub Actions workflow named "CD" automates building and packaging a VS Code extension. It triggers on pushes to Sequence Diagram(s)sequenceDiagram
autonumber
participant GH as GitHub (trigger)
participant R as Actions runner (ubuntu-latest)
participant Repo as Repository
participant Node as Node 22.x env
participant Art as Artifact storage / Job Summary
Note over GH,R #D6EAF8: Triggers: push(main), pull_request(main), workflow_dispatch
GH->>R: start "CD" job (package)
R->>Repo: actions/checkout
R->>Node: setup-node (22.x, npm cache, registry)
R->>Node: npm ci
R->>Node: npm install -g @vscode/vsce
R->>Repo: read package.json -> extract version
R->>Repo: determine branch or PR ref -> sanitize for filename
R->>Node: npm run package (outputs vscode-deepnote-insiders.vsix)
Node-->>R: .vsix (dist/)
R->>R: rename .vsix -> includes version + sanitized-branch
R->>Art: upload-artifact (name includes version+branch)
R->>Art: add-job-summary (version, branch, install instructions)
Note over Art,R #E9F7EF: Artifact + summary available in workflow UI
Suggested reviewers
Pre-merge checks✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #79 +/- ##
=====================================
Coverage 71% 71%
=====================================
Files 523 523
Lines 39001 39001
Branches 4905 4905
=====================================
Hits 27834 27834
Misses 9542 9542
Partials 1625 1625 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/package.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/package.yml
55-55: "github.head_ref" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/reference/security/secure-use#good-practices-for-mitigating-script-injection-attacks for more details
(expression)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build & Test
- GitHub Check: Lint & Format
- GitHub Check: Build & Package Extension
🔇 Additional comments (1)
.github/workflows/package.yml (1)
70-75: No changes required; review comment is factually incorrect.The hardcoded filename
ms-toolsai-jupyter-insiders.vsixin the workflow is correct. Thenpm run packagescript explicitly outputs to this name via the-oflag inpackage.jsonline 2129:vsce package -o ms-toolsai-jupyter-insiders.vsix. Themvcommand will succeed without modification.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/package.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/package.yml
55-55: "github.head_ref" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/reference/security/secure-use#good-practices-for-mitigating-script-injection-attacks for more details
(expression)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build & Test
- GitHub Check: Lint & Format
- GitHub Check: Build & Package Extension
🔇 Additional comments (1)
.github/workflows/package.yml (1)
70-75: No changes needed—the VSIX filename is explicitly set in the build command.The
vsce packagecommand in package.json uses the-o ms-toolsai-jupyter-insiders.vsixflag, which guarantees that exact output filename. The comment accurately describes what the package script produces, and themvcommand will succeed. The filename is not a guess or copy-paste artifact.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/package.yml (1)
56-68: Fix script injection vulnerability withgithub.head_ref.Line 61 directly interpolates untrusted user input (
github.head_ref) into the shell script, enabling script injection attacks. Pass it through an environment variable first (as flagged by actionlint).- name: Extract and sanitize branch name id: branch-name + env: + HEAD_REF: ${{ github.head_ref }} run: | # Get branch name from ref if [[ "${{ github.event_name }}" == "pull_request" ]]; then - BRANCH="${{ github.head_ref }}" + BRANCH="$HEAD_REF" else BRANCH="${GITHUB_REF#refs/heads/}" fi # Sanitize branch name for filename (replace / with -) SAFE_BRANCH=$(echo "$BRANCH" | sed 's/\//-/g') echo "branch=$SAFE_BRANCH" >> $GITHUB_OUTPUT echo "Branch name: $BRANCH (sanitized: $SAFE_BRANCH)"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/package.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/package.yml
58-58: "github.head_ref" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/reference/security/secure-use#good-practices-for-mitigating-script-injection-attacks for more details
(expression)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build & Package Extension
- GitHub Check: Build & Test
🔇 Additional comments (1)
.github/workflows/package.yml (1)
73-78: The hardcoded filename is correct—no issue here.The
packagescript explicitly invokesvsce package -o ms-toolsai-jupyter-insiders.vsix, so themvcommand will find and rename the file without failure. The filename is the configured build artifact, not a misplaced reference.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/package.yml (1)
43-44: Consider alternatives to global vsce installation.Installing
@vscode/vsceglobally reduces CI reproducibility and makes the version implicit. Two better approaches:Option 1 (minimal): Use
npx:- - name: Install vsce - run: npm install -g @vscode/vsce - - name: Package extension - run: npm run package + run: npx @vscode/vsce packageOption 2 (recommended): Add to
devDependenciesinpackage.json, then update npm script:- - name: Install vsce - run: npm install -g @vscode/vsce - - name: Package extension run: npm run package(and update
package.jsonto have"package": "vsce package"in scripts)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (2)
icon.pngis excluded by!**/*.pngpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
.github/workflows/package.yml(1 hunks)
🔇 Additional comments (1)
.github/workflows/package.yml (1)
53-68: Good: script injection mitigation properly applied.The branch name extraction correctly uses environment variables to avoid direct shell interpolation of
github.head_ref, and sanitization usesprintfsafely. This addresses the prior actionlint security findings.
Artmann
requested review from
andyjakubowski,
jamesbhobbs and
saltenasl
as code owners
jamesbhobbs
left a comment
jamesbhobbs
left a comment
There was a problem hiding this comment.
I have quibbles but the workflow looks fine, I haven't tested the packaging works, I assume the author did.
Fixes #
Summary by CodeRabbit